miopen: init at 2.18.0-5.3.3 #202476
miopen: init at 2.18.0-5.3.3 #202476
Conversation
Honestly it seems a little redundant to me, but then I think we're going to have to do something similar for pytorch anyway, so alright. |
|
On second thought, miopen should be the base and just toggle |
| ] ++ lib.optionals buildDocs [ | ||
| latex | ||
| doxygen | ||
| sphinx |
There was a problem hiding this comment.
use SphinxHook, build the documentation unconditionally and enable the doc and/or man outputs. They can be installed per user-choice through the documentation module.
There was a problem hiding this comment.
Many of the other ROCm derivations I've made do the same, why use SphinxHook here?
The others also don't build the documentation unconditionally, so again, why here?
There was a problem hiding this comment.
Because I first spotted it here. It's a recommendation to simplify things, not a precondition to get things merged.
There was a problem hiding this comment.
Not having this built on hydra is rough and makes it obvious why parts of the build were made conditional.
There was a problem hiding this comment.
Okay makes sense.
It might be a little bit before I commit the new changes since I gotta rebuild everything.
There was a problem hiding this comment.
There are a few other derivations that build documentation so it might be easier to merge this as-is and then I'll submit a rocm-related PR concerning sphinxHook.
The directories are all over the place for ROCm, so I'm not entirely sure one solution will work for all of them.
There was a problem hiding this comment.
Ah right, need to fix the docs review before merge. One moment.
There was a problem hiding this comment.
After some testing, simplifying this with sphinxHook isn't a good option.
The documentation seems to depend on certain variables from cmake.
Here's what I have done: Madouura@f4ae10e
There was a problem hiding this comment.
We also lose the ability to reasonably generate the pdf.
There was a problem hiding this comment.
Clarification: The documentation does build, but will silently error in the actual documents.
You'll get missing parts and messed up in-lines.
| , zlib ? null | ||
| , fetchurl ? null | ||
| , buildDocs ? false | ||
| , buildTests ? false |
There was a problem hiding this comment.
They are not needed for use as a dependency or for end-user functionality.
They can also be costly to build.
There was a problem hiding this comment.
They are probably important to check whether the packaging works out alright. Not sure what "can also be costly" means vs. being actually costly.
There was a problem hiding this comment.
Most tests don't run within the nix sandbox, also.
There was a problem hiding this comment.
It's also the reason why #200757 exists.
Tests will need to be run in an impure fashion.
There was a problem hiding this comment.
There should not be ? null heres regardless if the tests are run or not, if they are currently broken or not.
ofborg
bot
added
the
11.by: package-maintainer
Description of changes
Tracking: #197885
I wanted to get either a way to generate KDBs or more of them added to https://repo.radeon.com/rocm/miopen-kernel/rel-5.0/, but I don't think that's gonna happen for a while. So, doing the PR now.
Things done
sandbox = trueset innix.conf? (See Nix manual)nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)nixos/doc/manual/md-to-db.shto update generated release notes